Skip to content

add APIs and enums for cl_intel_unified_shared_memory#65

Merged
bashbaug merged 4 commits intoKhronosGroup:masterfrom
bashbaug:add-cl_intel_unified_shared_memory
Jun 9, 2020
Merged

add APIs and enums for cl_intel_unified_shared_memory#65
bashbaug merged 4 commits intoKhronosGroup:masterfrom
bashbaug:add-cl_intel_unified_shared_memory

Conversation

@bashbaug
Copy link
Contributor

@bashbaug bashbaug commented Jan 21, 2020

Adds APIs and enums for the cl_intel_unified_shared_memory extension. The draft extension spec can be found here.

Also, fixes a few other minor issues:

  1. Updated copyright dates to 2020.
  2. Removed extraneous semi-colons (supersedes Remove unnecessary semicolons #53).
  3. Changed a few C++ comments (//) to C comments (/* */).

kpet
kpet reviewed Jan 23, 2020
View reviewed changes
Copy link
Contributor

@kpet kpet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me after a first quick scan. I couldn't find the changes to the XML though. Did you push them to a specific branch?

@bashbaug
remove CL_MEM_ALLOC_DEFAULT_INTEL
10082d7 
This value is not in the latest rev of the spec.
@bashbaug
add aliases for cl_mem_migration_flags
58c6a2b 
Fixes compilation pre-OpenCL 1.2, where cl_mem_migration_flags is not defined.
@bashbaug bashbaug mentioned this pull request Jan 23, 2020
Merged
@bashbaug
Copy link
Contributor Author

bashbaug commented Jan 23, 2020

Thanks!

I just created a PR to update the XML file here: KhronosGroup/OpenCL-Docs#192

I've also fixed the headers so CI builds are passing when compiling pre-OpenCL 1.2 (thanks for setting this up!).

alycm
alycm previously approved these changes Jan 26, 2020
View reviewed changes
Copy link
Contributor

@alycm alycm left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed this along with KhronosGroup/OpenCL-Docs#192, LGTM.

@bashbaug
Copy link
Contributor Author

bashbaug commented Feb 24, 2020

Ping... is anything more needed for this PR? The XML changes have already been merged.

kpet
kpet reviewed Feb 24, 2020
View reviewed changes
Copy link
Contributor

@kpet kpet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with that change but some bitfield definitions seem to be missing from the XML.

@bashbaug bashbaug changed the title add APIs and enums for cl_intel_unified_shared_memory WIP: add APIs and enums for cl_intel_unified_shared_memory Feb 28, 2020
@bashbaug
Copy link
Contributor Author

bashbaug commented Feb 28, 2020

I'm looking into options that do not require defining aliases for the cl_mem_migration_flags type and enums, so I've marked this PR WIP for now.

Related spec issue: https://github.com/KhronosGroup/OpenCL-Docs/issues/214

@alycm alycm mentioned this pull request Mar 12, 2020
Closed
@bashbaug
for now, remove aliases to cl_mem_migration_flags
09a4cef 
Instead of adding aliases to cl_mem_migration_flags and associated
enums, restrict the clEnqueueMigrateMemINTEL API to OpenCL 1.2 or
newer, which satisfies all current use-cases.

If needed, the aliases can be re-introduced at a later date, since
adding them does not change the ABI.
@CLAassistant
Copy link

CLAassistant commented May 18, 2020 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@bashbaug bashbaug changed the title WIP: add APIs and enums for cl_intel_unified_shared_memory add APIs and enums for cl_intel_unified_shared_memory May 19, 2020
@bashbaug
Copy link
Contributor Author

bashbaug commented May 19, 2020

To get this PR un-stuck for now, let's back out the cl_mem_migration_flags_intel aliases and instead require OpenCL 1.2 headers for clEnqueueMigrateMemINTEL so we can reuse the existing cl_mem_migration_flags type and enums. This covers all of our current USM devices, and we can always add aliases later for pre-1.2 devices if needed, pending the resolution of KhronosGroup/OpenCL-Docs#214.

I will create a PR to back out cl_mem_migration_flags_intel from the XML file, so it remains consistent with the headers.

I've removed WIP and I believe this is ready for review and merging. Thanks!

@bashbaug bashbaug mentioned this pull request May 19, 2020
Merged
alycm
alycm approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@alycm alycm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (again), the XML changes removing cl_mem_migration_flags_intel are already merged.

@bashbaug
Copy link
Contributor Author

bashbaug commented Jun 9, 2020

Thanks! Merging.

@bashbaug bashbaug merged commit b04034a into KhronosGroup:master Jun 9, 2020
@bashbaug bashbaug deleted the add-cl_intel_unified_shared_memory branch June 9, 2020 17:49
@SunSerega SunSerega mentioned this pull request Sep 8, 2022
Merged
EwanC pushed a commit to EwanC/OpenCL-Headers that referenced this pull request Oct 4, 2024
@bashbaug
fix freebsd warnings (KhronosGroup#65)
fb23b75
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kpet kpet kpet left review comments

+1 more reviewer

@alycm alycm alycm approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bashbaug @CLAassistant @kpet @alycm